Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: Sync to Linux 6.12 syscall definitions #435

Closed
wants to merge 3 commits into from

Conversation

xen0n
Copy link
Contributor

@xen0n xen0n commented Aug 20, 2024

The arch-syscall-validate script updates will need to be backported to v2.5.x later, for continuing to sync aarch64 and riscv64 that got refactored to source the syscall definitions from a syscall.tbl instead of asm-generic/unistd.h.

@coveralls
Copy link

coveralls commented Aug 20, 2024

Coverage Status

coverage: 89.454%. remained the same
when pulling d909801 on xen0n:linux-6.11-syscalls
into 9c4eb5e on seccomp:main.

@xen0n xen0n changed the title Sync to Linux 6.11 syscall definitions RFE: Sync to Linux 6.11 syscall definitions Aug 20, 2024
@xen0n
Copy link
Contributor Author

xen0n commented Aug 20, 2024

After the change, linux-headers >= 6.11 will be required for native LoongArch all builds, for definitions of new syscalls introduced in this version.

@lixing-star
Copy link

After the change, linux-headers >= 6.11 will be required for native LoongArch all builds, for definitions of new syscalls introduced in this version.

this is really helpful for LoongArch, thanks.

@Dandan336
Copy link

Hi maintainers,

  • About defining __ARCH_WANT_NEW_STAT in unistd.h
    'LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h' 1 was merged in linux upstream 2.
    'LoongArch: Define __ARCH_WANT_NEW_STAT in unistd.h' was supported in the stable version of linux, as follows,
  1. branch linux-6.1.y
    https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.1.y&id=68a35d0abf8059ee41f164093c700297343101b2
  2. branch linux-6.6.y
    https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.6.y&id=9d91b004df9a6aa2487661e52a398fd808184955
  3. branch linux-6.10.y
    https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.10.y&id=268a625399c63083aeb9c4fc818f5f2d916d5adb
  4. branch linux-6.11.y
    https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.11.y&id=7697a0fe0154468f5df35c23ebd7aa48994c2cdc
  • The results of the test cases
    Based on main branch of libseccomp, I pulled this pull-request 'RFE: Sync to Linux 6.11 syscall definitions' submitted by xen0n. I built libseccomp project on Debian loong64 rootfs (the verson of linux-libc-dev is 6.11.2-1), there is 0 failed or errored test case.The results of the test cases are as follows,
Regression Test Summary
 tests run: 5084
 tests skipped: 124
 tests passed: 5084
 tests failed: 0
 tests errored: 0
=================================
PASS: regression
=============
1 test passed
=============

Please review.

@yetist
Copy link
Contributor

yetist commented Oct 28, 2024

@drakenclimber @pcmoore

Could you review this PR? Thanks.
Also, I would like to know when v2.6.0 will be released.

@rusty-snake
Copy link
Contributor

Also, I would like to know when v2.6.0 will be released.

https://github.com/seccomp/libseccomp?tab=readme-ov-file#release-process

TL;DR: When it's ready.

#else
#define __SNR_map_shadow_stack __PNR_map_shadow_stack
#endif

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't map_shadow_stack() defined for all the arches?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is, but in this commit the PNR was allocated anyway and the change is already shipped in v2.5.5. So if we're to consider PNR values as part of the public API we would have to keep __PNR_map_shadow_stack defined, and for stylistic consistency we would have to keep this #ifdef-fery as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder @xen0n.

In the manpages we "highly recommend" the use of the SCMP_SYS() macro as opposed to directly specifying syscall numbers so it's a bit of a gray area. I think it would be okay for us to modify/remove the PNR syscall values if necessary, but we probably shouldn't do so unless we have a good reason. Considering everything, it looks like you approach of leaving the PNR values as-is is likely the right one. Thanks :)

@pcmoore
Copy link
Member

pcmoore commented Oct 30, 2024

Just one small comment/clarification/question above, but otherwise I think this looks good. Thanks for all your work on this @xen0n!

@@ -280,6 +280,9 @@
#define __PNR_atomic_barrier -10246
#define __PNR_atomic_cmpxchg_32 -10247
#define __PNR_getpagesize -10248
#define __PNR_map_shadow_stack -10249
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR: The PNR numbers below -10244 (__PNR_memfd_secret) have already diverged between libseccomp:main and libseccomp:release-2.5.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a FYI, I'm okay with fixing that sort of thing while merging the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have preserved the __PNR_map_shadow_stack position while rebasing.

Apart from de-duplication of logic, this refactor is also going to help
syncing to the Linux 6.11+ definitions, where all architectures are
converted to source their syscall definitions from syscall.tbl files.

The change is tested on Linux 6.2 sources to not affect the generated
syscalls.csv apart from timestamp changes.

Signed-off-by: WANG Xuerui <[email protected]>
The aarch64, loongarch64 and riscv64 architectures have their syscall
table sources changed to scripts/syscall.tbl, from the original
inclusion of asm-generic/unistd.h. Make the script recognize the new
format for these architectures.

Signed-off-by: WANG Xuerui <[email protected]>
Due to the addition of fstat & newfstatat to the LoongArch syscall ABI,
tests 38 and 55 have to be updated for the changed syscall numbers.

As for the PNR additions, normally they are allocated alphabetically for
the syscalls introduced between updates of the table, but in the v2.5
release branch -10245 is already assigned to map_shadow_stack in
commit 53267af ("all: update the syscall table for Linux v6.7-rc3").
While the map_shadow_stack syscall is in fact available across all
architectures, for consistency with v2.5.5 and later it is kept in the
same position in this update.

Signed-off-by: WANG Xuerui <[email protected]>
@xen0n xen0n changed the title RFE: Sync to Linux 6.11 syscall definitions RFE: Sync to Linux 6.12 syscall definitions Oct 31, 2024
@xen0n
Copy link
Contributor Author

xen0n commented Oct 31, 2024

This is now rebased and updated to reflect Linux v6.12-rc5 syscall info (no new syscalls since 6.11). I have also incorporated @rusty-snake's review comment of keeping PNR allocations consistent with v2.5.x. Tests still pass on aarch64, loongarch64, riscv64 and x86_64.

@drakenclimber
Copy link
Member

My apologies for being quiet the last few weeks. I've had some other high-priority work on my plate, but I'm thinking I should have some time in the next few weeks to get back into libseccomp work. Thanks all for your patience :).

@pcmoore
Copy link
Member

pcmoore commented Nov 8, 2024

This looked good to me and it passed all my tests locally so I merged the PR at HEAD f01e675, thanks for you work and patience @xen0n!

@drakenclimber nothing in here looked controversial and we needed something like this for future syscall updates so I went ahead and merged the PR, if you notice anything awry please let me know and we can revert/fix it.

@pcmoore
Copy link
Member

pcmoore commented Nov 8, 2024

While the PR is merged, I'm going to leave this open until I finish the backport to the release-2.5 branch.

@pcmoore
Copy link
Member

pcmoore commented Nov 11, 2024

Here is the release-2.5/v2.5.x branch backport: #439

@pcmoore
Copy link
Member

pcmoore commented Nov 11, 2024

With the backport PR now posted, I'm closing this PR, thanks again everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants